Skip to content

Fixed 'Render and Build' test#21

Merged
Arctis-Fireblight merged 1 commit intoFireblight-Studios:mainfrom
Arctis-Fireblight:fix-unit-tests
Jan 9, 2026
Merged

Fixed 'Render and Build' test#21
Arctis-Fireblight merged 1 commit intoFireblight-Studios:mainfrom
Arctis-Fireblight:fix-unit-tests

Conversation

@Arctis-Fireblight
Copy link
Copy Markdown

@Arctis-Fireblight Arctis-Fireblight commented Jan 9, 2026

Test now looks for the rendered elements seperately to account for differences in serialization, since the exact output of CalendarFull.Render() can vary a little based on the exact date.
Fixes #16

Summary by CodeRabbit

  • Tests
    • Refactored test assertions for improved clarity and maintainability.

✏️ Tip: You can customize this high-level summary in your review settings.

Test now looks for the rendered elements seperately to account for differences in serialization, since the exact output of `CalendarFull.Render()` can vary a little based on the exact date.
@Arctis-Fireblight Arctis-Fireblight added this to the v2.5.3 milestone Jan 9, 2026
@Arctis-Fireblight Arctis-Fireblight self-assigned this Jan 9, 2026
@Arctis-Fireblight Arctis-Fireblight added the bug Something isn't working label Jan 9, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

The test file calendar-full.test.ts was refactored to split a single complex assertion for the month-year span element into three separate assertions, independently verifying the data-visible attribute, textual content, and base class presence.

Changes

Cohort / File(s) Summary
Test Assertion Refactoring
src/classes/renderer/calendar-full.test.ts
Split combined month-year span assertion into three independent checks: data-visible attribute value, textual content (month and year), and base class "fsc-month-year" presence

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Fixed 'Render and Build' test' is partially related to the changeset, referring to the specific test file being modified, but is overly broad and doesn't clarify what aspect of the test was fixed or why. Consider a more specific title such as 'Refactor CalendarFull.Render test to separate concerns' or 'Fix Render and Build test to handle serialization variability' to better convey the nature of the fix.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses issue #16 by fixing a failing test, but the changes only refactor one test assertion rather than comprehensively resolving the underlying date-dependency issues mentioned in the issue.
Out of Scope Changes check ✅ Passed All changes are confined to the test file and directly address the failing test mentioned in issue #16, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/classes/renderer/calendar-full.test.ts:
- Around line 57-59: The test relies on real time because
PredefinedCalendar.setToPredefined(tCal, PredefinedCalendars.Gregorian) sets
currentDate to new Date(); change the setup to inject a fixed deterministic date
(e.g., create a Date for a known month/year and assign it to tCal.currentDate or
use a PredefinedCalendar helper that accepts a seed date) so
CalendarFull.Render() always outputs the same month/year; also replace the
brittle partial class assertion on the rendered output (currently expecting
`class="fsc-month-year`) with a full HTML fragment match consistent with lines
63-77 (match the complete element/class string produced by CalendarFull.Render
instead of a substring).
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3867f99 and bde4bfe.

📒 Files selected for processing (1)
  • src/classes/renderer/calendar-full.test.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-08T08:30:38.842Z
Learnt from: Arctis-Fireblight
Repo: Fireblight-Studios/foundryvtt-simple-calendar PR: 20
File: src/classes/applications/configuration-app.ts:1870-1876
Timestamp: 2026-01-08T08:30:38.842Z
Learning: In the Fireblight-Studios/foundryvtt-simple-calendar fork, when importing journal entries from the legacy 'foundryvtt-simple-calendar' module that already exist in the world, prefer creating new journal entries with new IDs instead of updating existing ones. This avoids potential issues with lingering or 'dirty' legacy configuration data. Apply this approach specifically in the configuration/import logic (e.g., in src/classes/applications/configuration-app.ts) and ensure that existing entries are left untouched unless a mechanism explicitly clones and re-links references.

Applied to files:

  • src/classes/renderer/calendar-full.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

@Arctis-Fireblight Arctis-Fireblight merged commit 4c1e384 into Fireblight-Studios:main Jan 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Unit Tests failing again

1 participant